feat(rum): add sessionReplayOnErrorSampleRate - #24
Open
Fiona2016 wants to merge 7 commits into
Open
Conversation
…export from mockProfiler imported registerCleanupTask and getGlobalObject from the rum package rather than core, and profiler.spec.ts imported from package names this repository does not publish. Since mockProfiler is re-exported from the rum test barrel, the broken imports took every spec that touches that barrel down with them - around 220 tests never ran.
A session drawn by this rate records from the start but uploads nothing until it reports an error. If none ever happens, nothing is sent and the session is never stored. On the first error the withheld buffer is released and recording continues normally, so the replay covers what led up to the error rather than starting at it. The buffer is bounded on both axes. Time: a buffer that spans more than a minute is dropped and restarted from a fresh full snapshot, so what is released stays a minute at most. Size: the existing segment byte limit still applies while withheld, and restarts are spaced out so that a document whose full snapshot alone exceeds that limit degrades instead of restarting in a loop. A withheld buffer belongs to the session that produced it. It is released only when that same session reports the error - if the session expires or is renewed first, the records are dropped, so an expiry can never turn into an upload for a session that never errored. Buffers that are dropped roll back their replay stats, and has_replay is not reported while a replay is being withheld, so neither the counters nor the link offer a replay that does not exist. Errors raised by the SDK about its own transport do not release anything: those are our failures, not the application's, and counting them would make every session an error session wherever our endpoint is unreachable.
The flush path derived the same thing twice under two names, and the mapping of the internal checkout reason onto a schema value only ever had one caller.
A page-exit rotation used to throw the buffer away, and with it the full snapshot a released replay has to start from - everything recorded afterwards is incremental and cannot be played on its own. Switching tabs raises this exit, and the page comes straight back, so an error reported after that would have released a replay that renders as good as nothing until the next view. Nothing can be sent while withheld, so there was never anything to gain from the rotation. A page that is really unloading takes the buffer with it either way.
…ased buffer Only the rotation notices that the withheld replay has been released, so a session that expires within one rotation of its own error still loses what the error had earned. Closing it would mean asking the session manager on every record.
The rollback that gives a dropped buffer's index_in_view back only lands when the encoder finishes, which is always a turn later. Restarting from a fresh full snapshot emitted records right away, so the next segment took its index before the rollback arrived - and once that session errored, two uploaded segments claimed the same index within one view while nothing claimed the first. Any error session that spends a minute on one view before erroring hit it. The restart now happens where the rollback lands. Also corrects a comment: a session expiring right after its own error does not lose the buffer. The history entry is still open when the recorder is stopped, so the stop flush sees the session as released and sends.
4 tasks
Without it, a replay collected under this rate is indistinguishable from one collected unconditionally once it has been uploaded - the two cost differently and answer different questions, and nothing downstream could tell them apart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A session is stored, and charged for, as a whole. That makes "keep every error, sample everything else" awkward to express by filtering events: dropping individual events still leaves the session stored, so the thing that actually counts does not go down.
sessionReplayOnErrorSampleRateexpresses it where it counts. A session drawn by this rate records a replay from the start but uploads nothing. If it never reports an error, nothing is ever sent and the session is never stored. On the first error the withheld recording is released, so the replay covers what led up to the error rather than starting at it.Changes
sessionReplayOnErrorSampleRate, drawn only for sessions the plainsessionReplaySampleRatedraw missed, so a session is never counted by both rates. It adds one tracking type and aBUFFERED_ON_ERRORreplay state.startSessionReplayRecordingManuallynow defaults tofalsewhen either replay rate is set: a session that has to be recording before the error cannot wait to be started by hand.has_replaynor the replay link is offered while a replay is withheld, so nothing points at a replay that does not exist.error.source === 'agent') release nothing: an unreachable intake should not turn every session into an error session.Also included: the profiling test helpers imported from package names this repository does not publish. Because the helper is re-exported from the rum test barrel, those imports stopped roughly 220 specs from loading at all; fixing them brings the suite back.
Test instructions
yarn test:unit. New specs cover withholding, release, the checkout window, the byte limit, the stats rollback, hidden pages, and session expiry and renewal.By hand, against
yarn dev:initwithsessionSampleRate: 100,sessionReplaySampleRate: 0,sessionReplayOnErrorSampleRate: 100, then callstartSessionReplayRecording().has_full_snapshot: truewith records reaching back before the error.Driven through Playwright against the same page, a 108 second session uploaded nothing before the error and then released a segment reaching back 48 seconds, with the full snapshot present.
Checklist